New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update coalesce precedence #11017
Update coalesce precedence #11017
Conversation
The Flow failure is related to this PR
@@ -139,22 +139,22 @@ export const types: { [name: string]: TokenType } = { | |||
tilde: new TokenType("~", { beforeExpr, prefix, startsExpr }), | |||
pipeline: createBinop("|>", 0), | |||
nullishCoalescing: createBinop("??", 1), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JLHwung If you follow the specs the precedence values is wrong :) The '??' should have lowest value.
There are a lot of docs and issue tickets about this, but here you can see it clearly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That precedence statement does not follow from the grammar notation in the spec
ShortCircuitExpression[In, Yield, Await]:
LogicalORExpression[?In, ?Yield, ?Await]
CoalesceExpression[?In, ?Yield, ?Await]
That statement was updated in tc39/proposal-nullish-coalescing@99ff664 where a NullishExpression was introduced
NullishExpression[In, Yield, Await, Nullish] :
LogicalORExpression[?In, ?Yield, ?Await, ?Nullish]
NullishExpression[?In, ?Yield, ?Await, +Nullish] `??` LogicalORExpression[?In, ?Yield, ?Await, +Nullish]
apparently here ??
is lower than ||
in this notation. But later it was changed to ShortCircuitExpression
, where
- the RHS of
CoalesceExpression
is eitherCoalesceExpression
orBitwiseORExpression
. - the RHS of
LogicalORExpression
does not containCoalesceExpression
So the grammar itself does not conclude that the precedence of coalesce
is higher or lower than the logical_or
. The only thing we know is that
both coalesce
and logical_or
are lower than bit_or
. (f.1)
logical_or
is lower than logical_and
. (f.2)
Since the precedence of coalesce
and logical_or
is meant to solve the error of mixing coalesce and logical operators and now we throw when they are mixed, I don't think the exact precedence of coalesce
and logical_or
will have any impact as long as the precedence specification satisfies (f.1) and (f.2).
I can think of one advantage if you specify the precedence as coalesce: 1, or: 2, and: 3
: you can now check if logical
is mixed with coalesce
by
(op.binop >> 1) ^ (nextOp.binop >> 1) === 1
This is similar to what is done in seafox but I think here code readability weighs more than performance, unless we do identify that parsing coalesce is the bottleneck.
9c3685f
to
87e058e
Compare
Co-authored-by: Toru Nagashima <public@mysticatea.dev>
87e058e
to
6b2c9e8
Compare
The first commit is part of my holiday parser refactor series. The new checking implements as @KFlash suggested in https://github.com/babel/babel/pull/10269/files/40eb40e34b689096d6a5921c63577904a187d4be#r313937603
The second commit updates the precedence to be align with the spec: https://tc39.es/ecma262/#prod-ShortCircuitExpression